Skip to content

Upgrade RuboCop related gems #3367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vinistock
Copy link
Member

@vinistock vinistock commented Mar 28, 2025

Motivation

Upgrade the rest of the outdated development gems, all related to RuboCop. There's a bug fix for the RuboCop add-on in these upgrades, so that we stop seeing an argument error while developing.

@vinistock vinistock self-assigned this Mar 28, 2025
@vinistock vinistock added the chore Chore task label Mar 28, 2025 — with Graphite App
Copy link
Member Author


How to use the Graphite Merge Queue

Add the label graphite-merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vinistock vinistock marked this pull request as ready for review March 28, 2025 18:10
@vinistock vinistock requested a review from a team as a code owner March 28, 2025 18:10
Comment on lines +114 to +118
# Maintain the original backtrace so that debugging cops that are breaking is easier, but re-raise as a
# different error class
internal_error = InternalRuboCopError.new(error)
internal_error.set_backtrace(error.backtrace)
raise internal_error
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While trying to understand if the bug I was seeing was in our code or in RuboCop's, I noticed that we were discarding the original backtrace for internal RuboCop errors.

That's not good, because we want to help people fix bugs in cops and not just point the backtrace to our own code. Here I changed to use set_backtrace to ensure that the original backtrace is preserved, which helps debugging problematic cops.

@@ -452,10 +452,10 @@ suite("Client", () => {
);

const expected = [
"# typed: strict",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were some changes in regards to the safety of some auto corrects. I changed the example, just to ensure that formatting actually runs on this example.

@vinistock vinistock force-pushed the 03-28-upgrade_rubocop_related_gems branch 6 times, most recently from 1bf88c8 to dc3352c Compare March 31, 2025 17:20
- rubocop-sorbet
- rubocop-md
- rubocop-minitest
- rubocop-rake

require:
- ./lib/rubocop/cop/ruby_lsp/use_language_server_aliases
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we migrate these as plugins?

@vinistock vinistock force-pushed the 03-28-upgrade_rubocop_related_gems branch from dc3352c to 5f936b3 Compare April 7, 2025 20:05
@vinistock
Copy link
Member Author

Just FYI, something in the new RuboCop versions is breaking our ability to show documentation links as part of the quick fixes, which is why I haven't shipped this yet.

I have not managed to figure out what changed, but it looks this call to base_url_for is not consistently returning nil.

I suspect that's a symptom and not the root cause. Something may have changed in how settings are taken into account.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Chore task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants